Fixing errors reported by mypy in search module files - query.py, commands.py and aggregation.py. #3666
Conversation
petyaslavova
force-pushed
the
fixing_mypy_errors
branch
from
ad1f6ea to
4a284e1
Compare
petyaslavova
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR fixes mypy type checking errors in the Redis search module by improving type annotations and correcting type inconsistencies across query.py, commands.py, and aggregation.py files.
- Added missing type imports and improved type annotations for better static type checking
- Fixed parameter type definitions to use proper Optional syntax instead of Union with None
- Corrected return type annotations and variable type declarations
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| redis/commands/search/query.py | Enhanced type annotations, fixed return types, and improved parameter type definitions |
| redis/commands/search/commands.py | Replaced Union with None syntax with proper Optional type annotations |
| redis/commands/search/aggregation.py | Added comprehensive type annotations and fixed method signatures |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| self._scorer: Optional[str] = None | ||
| self._filters: List = list() | ||
| self._ids: Optional[List[str]] = None | ||
| self._ids: Optional[Tuple[str]] = None |
There was a problem hiding this comment.
The type annotation Optional[Tuple[str]] creates a tuple with exactly one string element. This should likely be Optional[Tuple[str, ...]] for a variable-length tuple or Optional[List[str]] to maintain the original list behavior.
| self._ids: Optional[Tuple[str]] = None | |
| self._ids: Optional[Tuple[str, ...]] = None |
Copilot uses AI. Check for mistakes.
| self._args = args | ||
| self._field = None | ||
| self._alias = None | ||
| self._args: tuple[str, ...] = args |
There was a problem hiding this comment.
Use Tuple[str, ...] instead of tuple[str, ...] for consistency with other type annotations in the codebase that use the typing module imports.
Copilot uses AI. Check for mistakes.
|
|
||
| @property | ||
| def args(self) -> List[str]: | ||
| def args(self) -> tuple[str, ...]: |
There was a problem hiding this comment.
Use Tuple[str, ...] instead of tuple[str, ...] for consistency with other type annotations in the codebase that use the typing module imports.
Copilot uses AI. Check for mistakes.
| else: | ||
| # Chop off initial '@' | ||
| alias = self._field[1:] |
There was a problem hiding this comment.
The added else clause is unnecessary since the previous if block already handles the case when self._field is falsy. The original logic was correct - if self._field exists, process it; otherwise, the ValueError would have been raised.
| else: | |
| # Chop off initial '@' | |
| alias = self._field[1:] | |
| # Chop off initial '@' | |
| alias = self._field[1:] |
Copilot uses AI. Check for mistakes.
Pull Request check-list
Please make sure to review and check all of these items:
NOTE: these things are not required to open a PR and can be done
afterwards / while the PR is open.
Description of change
Fixing errors reported by mypy.